Refactor printing in CLI #931
Conversation
License: MIT Signed-off-by: Rasmus Erik Voel Jensen <[email protected]>
There was a problem hiding this comment.
Thank you for PR'ing this @rasmuserik ! 🙌🏽
will add too much boilerplate. Instead I just took the simplest approach, i.e. to instantiate an instance in the utils module, - and have a print function exported there.
Yes, makes sense.
If we want to have a quiet option, it could then be parsed once in bin.js, and then the logger might be instantiated from there.
I like that, could you move forward with it?
src/cli/commands/bootstrap/rm.js
Outdated
| @@ -24,7 +26,7 @@ module.exports = { | |||
| throw err | |||
| } | |||
|
|
|||
| list.Peers.forEach((l) => console.log(l)) | |||
| list.Peers.forEach(peer => print(peer)) | |||
There was a problem hiding this comment.
small nitpick. Change to (peer) => print(peer). Lint should have caught that.
There was a problem hiding this comment.
Fixed in latest version of the pull request :)
src/cli/utils.js
Outdated
| @@ -83,3 +83,4 @@ exports.createLogger = (visible) => { | |||
| } | |||
| } | |||
| } | |||
| exports.print = exports.createLogger(true) // TODO refactor/remove createLogger? | |||
License: MIT Signed-off-by: Rasmus Erik Voel Jensen <[email protected]>
Yep, I will do that :) |
|
The It is implemented as a verbosity level, |
|
Mind adding tests for the new modes @rasmuserik ? We just need:
No need for multiple levels of 'verbosity' |
|
I'll add a test for the quiet flag, |
License: MIT Signed-off-by: Rasmus Erik Voel Jensen <[email protected]>
|
done :) |
|
Thank you @rasmuserik !! :D |
* refactor: CLI should use print instead of console.log #495 License: MIT Signed-off-by: Rasmus Erik Voel Jensen <[email protected]> * feat: add quiet/verbose CLI flag, - related to #931 License: MIT Signed-off-by: Rasmus Erik Voel Jensen <[email protected]> * add test, and remove verbosity levels License: MIT Signed-off-by: Rasmus Erik Voel Jensen <[email protected]>
Excluding `ipfs-http-client` in the `browser` field of package.json was causing Meteor to not include the bundle in it's browser build. fixes #10411 License: MIT Signed-off-by: Alan Shaw <[email protected]>
This changes the CLI to use a logger instead of console.out, as discussed in #495
There is a design decision about how we instantiate the logger: originally there was a logger instantiated in command/init, but I think that instantiating it in every command, will add too much boilerplate. Instead I just took the simplest approach, i.e. to instantiate an instance in the
utilsmodule, - and have a print function exported there.If we want to have a quiet option, it could then be parsed once in
bin.js, and then the logger might be instantiated from there. Also if we intend always only have a single logger instance(?), we could removecreateLogger, and just have the print function, plus the ability to configure whether to silence it or not.